Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

feat: add quantitative testing #355

Merged
merged 19 commits into from
Oct 11, 2024
Merged

feat: add quantitative testing #355

merged 19 commits into from
Oct 11, 2024

Conversation

fzipi
Copy link
Member

@fzipi fzipi commented Sep 14, 2024 •
edited
Loading

what

  • 💡 add quantitative testing to the testing tool !
  • run using a local engine instead of parsing logs!
  • let the tool interface with the corpus to test, instead of pointing to a file
  • allow caching corpus files to ease CI/CD testing caching big files
  • plenty of variables and highly configurable from the cmd line
  • [EXPERIMENTAL] interface for adding more corpus
  • right now the tests are run against CRS mostly, but could be extended to additional rulesets

why

  • reuse the tool we use for testing and add more features
  • speed

future

  • get a threshold for knowing what is considered "bad" or "worse" than before
  • probably using go funcs to process corpus in parallel to lower times. right now is just line by line, which probably is really underperformant

@fzipi fzipi added the enhancement New feature or request label Sep 14, 2024
@fzipi fzipi force-pushed the feat/add-quantitative-testing branch from 3fc916a to 1dcebc6 Compare September 14, 2024 18:30
@fzipi fzipi requested a review from theseion September 14, 2024 19:15
@fzipi fzipi force-pushed the feat/add-quantitative-testing branch 3 times, most recently from 9ad2906 to 6de43c0 Compare September 14, 2024 23:37
@fzipi fzipi marked this pull request as ready for review September 14, 2024 23:41
@fzipi
feat: add quantitative testing
32bd0e9 
Signed-off-by: Felipe Zipitria <felipe.zipitria@owasp.org>
@fzipi fzipi force-pushed the feat/add-quantitative-testing branch from 6de43c0 to 32bd0e9 Compare September 19, 2024 12:38
theseion
theseion previously requested changes Sep 19, 2024
View reviewed changes
Copy link
Collaborator

@theseion theseion left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Really cool!

cmd/quantitative.go Outdated Show resolved Hide resolved
cmd/quantitative.go Outdated Show resolved Hide resolved
cmd/quantitative.go Outdated Show resolved Hide resolved
cmd/quantitative.go Outdated Show resolved Hide resolved
cmd/quantitative.go Outdated Show resolved Hide resolved
internal/quantitative/runner.go Outdated Show resolved Hide resolved
internal/quantitative/runner.go Outdated Show resolved Hide resolved
internal/quantitative/runner.go Outdated Show resolved Hide resolved
internal/quantitative/runner.go Outdated Show resolved Hide resolved
internal/quantitative/stats.go Show resolved Hide resolved
@fzipi @theseion
Apply suggestions from code review
d2fba4c 
Co-authored-by: Max Leske <250711+theseion@users.noreply.github.com>
@huberts90
Copy link
Collaborator

Could you please link some basic theoretical explanation to this issue to outline the motivation standing behind this PR.
Thanks in advance!

@fzipi
Copy link
Member Author

fzipi commented Sep 21, 2024

Well, I don't know if there is any "theoretical" explanation here, other than plain numbers.

We take a bunch of standard (meaning it doesn't contain attacks) text grabbed from the internet, and we run it against CRS. We get the percentage of the text that matches certain rules.

If you modify a rule and the numbers go up, your change will get more false positives. That's the gist of quantitative testing around rules.

@fzipi
feat: move to interfaces
ceb3e46 
Signed-off-by: Felipe Zipitria <felipe.zipitria@owasp.org>
@fzipi fzipi force-pushed the feat/add-quantitative-testing branch from df968f9 to b921abe Compare September 21, 2024 23:34
@fzipi
test: add more coverage
56d047d 
Signed-off-by: Felipe Zipitria <felipe.zipitria@owasp.org>
@fzipi fzipi force-pushed the feat/add-quantitative-testing branch from b921abe to 56d047d Compare September 22, 2024 00:21
@fzipi fzipi requested a review from theseion September 22, 2024 00:22
@fzipi
Copy link
Member Author

fzipi commented Sep 22, 2024

BTW, this is experimental until we have a good notion on what output we want from the tool.

@theseion
Copy link
Collaborator

@fzipi there are still two unresolved comments from the previous review.

@fzipi
fix: apply code review suggestions
9e30aa0 
Signed-off-by: Felipe Zipitria <felipe.zipitria@owasp.org>
@fzipi
Copy link
Member Author

fzipi commented Sep 22, 2024

@fzipi there are still two unresolved comments from the previous review.

I always fall in the hidden comments 🤦

@fzipi
docs: add basic documentation
fa547f8 
Signed-off-by: Felipe Zipitria <felipe.zipitria@owasp.org>
@fzipi
chore: cleanup comment
27a7aad 
Signed-off-by: Felipe Zipitria <felipe.zipitria@owasp.org>
@fzipi
fix: counting FPs
e8707f9 
Signed-off-by: Felipe Zipitria <felipe.zipitria@owasp.org>
@fzipi
fix: use both phases for getting FPs
300ba7d 
Signed-off-by: Felipe Zipitria <felipe.zipitria@owasp.org>
@fzipi
docs: add more examples
81f0e99 
Signed-off-by: Felipe Zipitria <felipe.zipitria@owasp.org>
@fzipi fzipi force-pushed the feat/add-quantitative-testing branch from d742042 to 81f0e99 Compare September 22, 2024 19:31
@fzipi fzipi dismissed theseion’s stale review September 22, 2024 19:31

All comments addressed.

@fzipi
fix: reduce noise in output
503bb77 
Signed-off-by: Felipe Zipitria <felipe.zipitria@owasp.org>
@fzipi
feat: add factories for creating new objects
b3232d5 
Signed-off-by: Felipe Zipitria <felipe.zipitria@owasp.org>
@fzipi
Copy link
Member Author

fzipi commented Sep 23, 2024

  • Added some factory methods
  • Cleaned up outputs
  • Added line number when printing payload in debug mode

fzipi and others added 2 commits September 22, 2024 22:13
@fzipi
test: simplify file tests
5b94d9a 
Signed-off-by: Felipe Zipitria <felipe.zipitria@owasp.org>
@fzipi
Merge branch 'main' into feat/add-quantitative-testing
32d8472
theseion
theseion requested changes Sep 25, 2024
View reviewed changes
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
internal/quantitative/local_engine.go Outdated Show resolved Hide resolved
internal/quantitative/local_engine.go Outdated Show resolved Hide resolved
@fzipi @theseion
fix: apply suggestions from code review
f3e4ed9 
Co-authored-by: Max Leske <250711+theseion@users.noreply.github.com>
@fzipi fzipi requested a review from theseion September 25, 2024 12:54
theseion
theseion previously approved these changes Oct 10, 2024
View reviewed changes
@fzipi fzipi requested a review from theseion October 10, 2024 19:27
@fzipi
fix: go.sum
0016c8f 
Signed-off-by: Felipe Zipitria <felipe.zipitria@owasp.org>
@fzipi fzipi merged commit 518e626 into main Oct 11, 2024
4 checks passed
@fzipi fzipi deleted the feat/add-quantitative-testing branch October 11, 2024 11:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@theseion theseion theseion approved these changes

Assignees
No one assigned
Labels
enhancement New feature or request
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@fzipi @huberts90 @theseion